This repository was archived by the owner on Jul 19, 2025. It is now read-only.
Configure different mass thresholds for identical & similar issues #47
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wfleming
force-pushed
the
will/threshold-config
branch
from
7ee54d8 to
e2c81cb
Compare
| To adjust this setting, add a `mass_threshold` key with your preferred value for | ||
| You can set thresholds for the two different types of duplication this engine reports: blocks that are identical to each other, and blocks that are structurally similar but differ in content. | ||
|
|
||
| To adjust these thresholds, you can add `identical_mass_threshold` and `similar_mass_threshold` keys with your preferred value for |
There was a problem hiding this comment.
Can you re-flow these paragraphs a bit better? I think the rest of this README (and ours in general) do hard-breaks at 80 columns.
In (I think default) vim,gipq will reflow the paragraph around the cursor with at your given textwidth, which I have at 80.
wfleming
force-pushed
the
will/threshold-config
branch
from
e2c81cb to
8c8a990
Compare
wfleming
force-pushed
the
will/threshold-config
branch
from
8c8a990 to
f17dc4a
Compare
|
We should probably get @brynary 's thoughts since I think this boils down to a Product decision. The code LGTM |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
One of the things I've noticed recently when running the duplication engine both against real code & just for testing is that generally when it emits issues I think are silly, it's because the the locations are structurally similar, but not identical. Examples of kind of silly issues this results in:
attributesmacro with two completely different sets of attribute names on two separate classes (i.e.attribute :foo, :bar, :bazis a duplicate ofattribute :fleek, :fluck, :fark)if (this.someProp) { a.doThing(); b.doAlso(); } else { a.doOtherThing(); b.doOtherAlso(); })I'm of the opinion that generally speaking the bar for similar code being a violation should be higher than identical code. Particularly at relatively low masses (which our defaults all are). Complaining about things like calling methods with different arguments in different places is not just low-value, but negative value IMHO: it's noise that distracts from legitimate issues & makes the engine as a whole seem less reliable.
This won't completely eliminate these instances, but it will make them rarer & easier to filter out without throwing out the baby with the bathwater (the baby in this case being more severe duplication issues that are either identical blocks or bigger similar blocks). I'm also starting to think setting per-file/global levels would also be helpful: i.e. you might want a lower threshold for similar code in the same file than you do for similar code across an entire project.
Thoughts, @codeclimate/review ?